Implementing NMF and Ataptive Thresholding #6
Conversation
contourusv/main.py
Outdated
|
|
||
| from preprocessing import clean_spec | ||
| from sklearn.decomposition import NMF, FastICA | ||
| from preprocessing import * |
There was a problem hiding this comment.
Same. Import only what you need.
There was a problem hiding this comment.
Fixed, will commit once all changes made.
| from generate_annotation import generate_annotations | ||
| from sklearn.exceptions import ConvergenceWarning | ||
| import warnings | ||
| warnings.simplefilter("ignore", ConvergenceWarning) |
There was a problem hiding this comment.
This seems a bit dangerous... unless you are sure that these can be safely ignored in your specific circumstances. And, even then, I would no make a general statement like this but rather put it in a with block only for the lines where I know I can ignore that warning.
contourusv/main.py
Outdated
| # Performs poor on Mouse_B6PUP | ||
| # Seems to take a long time to runimport warnings | ||
|
|
||
| model = NMF(n_components=30, init='nndsvd', random_state=0, max_iter=100) |
There was a problem hiding this comment.
What motivates this 30 value? Is there a justification for this value instead of some other values?
There was a problem hiding this comment.
That value was determined through trial and error. I tried several, (which I should have documented, but did not) and went with the one that produced the best results across all tests. I'll try a couple others and document outcomes.
contourusv/main.py
Outdated
| window = 'hann' | ||
| nperseg = 512 | ||
| noverlap = int(0.75 * nperseg) | ||
| noverlap = int(nperseg * 0.25) |
There was a problem hiding this comment.
The value 0.25 should be made a default value of a parameter of the function so that it is modifiable. Also, naming it as a variable will make its meaning clearer.
christian-oreilly
left a comment
christian-oreilly
left a comment
There was a problem hiding this comment.
@dw42CSCE Please have a look at the comments I added.
2 participants
This PR includes ContourUSV with adaptive thresholding and NMF implemented.
It also includes sphinx autodoccing hosted on a gh-pages branch, but this may need to be updated after the merge, specifically the global user email and username, to a user with permissions to update the repo directly. Currently it uses mine, but it will need to be updated to a repo owner.